fix!: Normalize Zelos connection indicators #9565
Conversation
gonfunko
force-pushed
the
zelos-highlights
branch
from
025b9ce to
b6db9fe
Compare
gonfunko
changed the title
github-actions
bot
added
PR: fix
|
I haven't looked at this in depth quite yet, but how are highlights now consistently brought to the front to fix the z-ordering issue, and does this introduce any problems with keyboard navigation? I remember digging into this before and I found that the standard Edit: Ah, #8981 was the one that may have made this work. I'm still curious about the first part, though, because I don't see |
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Other than my specific concerns on focus this seems like an excellent simplification.
| const highlightSvg = this.findHighlightSvg(); | ||
| if (highlightSvg) { | ||
| highlightSvg.style.display = ''; | ||
| highlightSvg.parentElement?.appendChild(highlightSvg); |
There was a problem hiding this comment.
This might cause focus loss problems. I'm also curious why this is necessary--shouldn't the highly SVG already be properly parented since it exists in the DOM at this stage?
Or is this the means to try and 'bring it to front'?
There was a problem hiding this comment.
Yes, this is the means to bring it to the front. Experimentally, there don't appear to be any focus issues, I think because the highlight SVG is not itself the thing that has focus; I was able to move blocks using keyboard nav and Zelos and it behaved as expected.
The basics
The details
Resolves
Proposed Changes
This PR cleans up Zelos' display of proposed connection indicators. Previously, proposed input connections indicators had three possible appearances:
Fuzzy, when an existing block would be replaced
Normal, when a block would be connected to an empty input
Skinny, same circumstances as above but when the highlight ring happened to be z-ordered below the fill for the input slot.
Likewise, next/previous connection indicators could appear normal, or partially obscured by the insertion marker depending on the whims of z-ordering:
Now:
BlockSvg.highlightShapeForInput()has been removed, as it was internal and had no callers.Breaking Changes
REPLACEMENT_GLOW_COLOUR,REPLACEMENT_GLOW_SIZE, andreplacementGlowFilterIdhave been removed fromBlockly.zelos.ConstantProvider. References to these fields should be removed; the appearance of the connection indicator can be styled with CSS targeting.blocklyHighlightedConnectionPath.IPathObject.updateReplacing()has been added; this method is optional and the rootPathObjectdefines it by simply toggling a class, so this should not strictly be breaking, but custom renderers may wish to implement/override it.